harden gbfgetcstr, gbfgetpstr. (#782)
authortsteven4 <13596209+tsteven4@users.noreply.github.com>
Sun, 5 Dec 2021 22:30:05 +0000 (15:30 -0700)
committerGitHub <noreply@github.com>
Sun, 5 Dec 2021 22:30:05 +0000 (15:30 -0700)
Hangs were detected with afl and the gdb reader and gbfgetcstr.  One such
hang reads a fuzzed 32 bit url count with FREAD_i32, and then proceeds
to fetch that many strings with FREAD_CSTR_AS_QSTR.  This results in
fetches from beyond EOF that are undetected.  gbfgetcstr_old is modified
to throw a fatal error if reading is attempted beyond EOF.

A similar failure was not detected with gbfgetpstr, but it is also
modified to throw a fatal error if reading is attempted beyond EOF.

gbfile.cc

index cba62e281b7f866c2ed0f4f1119ab4e3c3c63af5..b9f8c906c0cb22a02d3d6179b5207fbd6a44e25c 100644 (file)
--- a/gbfile.cc
+++ b/gbfile.cc
@@ -990,10 +990,14 @@ gbfgetcstr_old(gbfile* file)
   for (;;) {
     int c = gbfgetc(file);
 
-    if ((c == 0) || (c == EOF)) {
+    if (c == 0) {
       break;
     }
 
+    if (c == EOF) {
+      fatal("%s: Unexpected end of file (%s)!\n", file->module, file->name);
+    }
+
     if (len == file->buffsz) {
       file->buffsz += 64;
       str = file->buff = (char*) xrealloc(file->buff, file->buffsz + 1);
@@ -1030,17 +1034,20 @@ gbfgetnativecstr(gbfile* file)
 }
 
 /*
- * gbfgetpstr: Reads a pascal string (first byte is length) from file.
- *             The result is a temporary allocated entity: use it or free it!
+ * gbfgetpstr: Reads a pascal short string (first byte is length) from file.
  */
 
 QString
 gbfgetpstr(gbfile* file)
 {
   int len = gbfgetc(file);
+  if (len == EOF) {
+    fatal("%s: Unexpected end of file (%s)!\n", file->module, file->name);
+  }
   QByteArray ba;
   ba.resize(len);
-  gbfread(ba.data(), 1, len, file);
+  is_fatal((gbfread(ba.data(), 1, len, file) != (gbsize_t) len),
+           "%s: Unexpected end of file (%s)!\n", file->module, file->name);
 
   return QString(ba);
 }